✅ Deploy Preview for cerulean-figolla-1f9435 ready!

Name	Link
🔨 Latest commit	e930544
🔍 Latest deploy log	https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/6971fd78538f1500082f47d6
😎 Deploy Preview	https://deploy-preview-8002--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.48%. Comparing base (33d80db) to head (e930544).

@@            Coverage Diff             @@
##             main    #8002      +/-   ##
==========================================
- Coverage   73.54%   73.48%   -0.06%     
==========================================
  Files         237      237              
  Lines       35631    35631              
==========================================
- Hits        26204    26185      -19     
- Misses       7566     7582      +16     
- Partials     1861     1864       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Hi @zhaohuabing
Thanks for your effort - we would look forward to having such a feature!
IMHO, I would rather see a solution more like the one you described in the alternative approach.
I see three parts in your configuration:

1. ClientIP Source
2. GeoIP Database source
3. Access rules based on geo information

Client IP Source

In my opinion, this setting is redundant to [ClientIPDetectionSettings](https://gateway.envoyproxy.io/latest/api/extension_types/#clientipdetectionsettings) from ClientTrafficPolicy`.
Therefore, I wouldn't repeat that here and rely on this setting.
Or do you see a use-case that would make sense to have two separate settings?

Envoy allows the different XFF configuration for HCM and GeoIP filter, I don't know the exact reason, but there might be use cases that they could be different?

Huabing:
Envoy supports configuring XFF differently for HCM and the GeoIP filter. I’m not sure why, but there may be edge cases where they need to diverge.

One possible use case is that different routes may need different XFF settings (e.g., different source headers or different hop counts). That isn’t feasible with ClientTrafficPolicy today since it applies at the Gateway level, not per-route.

Another concern with this approach is that it would generate XFF configuration for both the HCM and the GeoIP filter. That could have unintended side effects in cases where HCM-level XFF configuration isn’t desired.

That said, I think consolidating this in one place in the control plane is a better UX and helps avoid misconfiguration. An optional XFF configuration in the SecurityPolicy should also be supported to allow route-level configuration.

ref:

• https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/http/geoip/v3/geoip.proto#envoy-v3-api-msg-extensions-filters-http-geoip-v3-geoip-xffconfig
• https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/http/original_ip_detection/xff/v3/xff.proto

GeoIP Database source

The database source is an infrastructure responsibility, and I don't see that this information should be provided for each SecurityPolicy. As an application owner, I just want to enable GeoIP access rules without the need to know where the DB is located.
Besides, you might want to have the geo information in the access logs, so this would also require configuring geolocation detection on a cluster level.
Therefore, I would see the option to enable a GeoIP Provider on a cluster level, for example, in the EnvoyProxy or EnvoyGatway spec.

Huabing:
Agree, it makes more sense to share the provider info across SPs.

Access rules

I think configuring access decision with geo location information as part of the SecurityPolicy.authorization rules would be the way to go. I agree with your concern about using the existing header principal. But what would you think about extending principal with geoLocation or clientLocation?

geolocation:
    countries: ["AB", "CD"]
    cities: []
    asns: []
    ...

This would ensure that the geo information population and the headers to match are entirely within the responsibility of the controller manager.

Huabing:
Yeah, this would work, and it allows using geo information with other conditions for more flexible access control.

What do you think?

Huabing:
All the suggestions are legitimate. One main concern I have is that GeoIP configuration would be spread across four places:

• EnvoyGateway / EnvoyProxy for providers
• ClientTrafficPolicy for source IP
• SecurityPolicy (GeoIP) for the provider reference
• SecurityPolicy (Authorization) for access control

That feels harder for users to understand and configure correctly.

Hi @zhaohuabing, do you know if this a PR that could be shipped with v1.8.0 ?

My team and I are currently benchmarking possible replacement of ingress-nginx, and Envoy Gateway has been a great candidate so far. Except on one mandatory feature for us that is GeoIP filtering (to deny countries under embargo). I know this is debatable in terms of real protection, but this is more of a legal matter and we can't do this without this feature.

BTW, thank you very much for your work (you personnaly and Envoy/Gateway teams in general) on this project !

@funkluk replied to your comment inline(text in italics) to make it easier to follow.

@sboulkour No guarantees, but aiming for v1.8.0 — and it looks very likely if we can agree on the API.

@zhaohuabing seems like we are on the same page, and I fully agree that the drawback is that the config is a bit more cluttered. But on the other hand, it allows IMHO a more flexible usage.

Two last remarks:

1. I'm not sure if we currently need more than one provider, as Envoy proxy has no other implementation than the MaxMind provider. I don't see a practical use-case of having two different database sets in the same gateway... This would also remove the requirement that we have to select the provider in the SecurityPolicy

2. IMHO I'd wait before implementing an XFF configuration on route-level until someone comes up with the actual need...